[pallet-revive] last call return data API #5779

xermicus · 2024-09-19T20:25:34Z

This PR introduces 2 new syscalls: return_data_size and return_data_copy, resembling the semantics of the EVM RETURNDATASIZE and RETURNDATACOPY opcodes.

The ownership of ExecReturnValue (the return data) has moved to the Frame. This allows implementing the new contract API functionality in ext with no additional copies. Returned data is passed via contract memory, memory is (will be) metered, hence the amount of returned data can not be statically known, so we should avoid storing copies of the returned data if we can. By moving the ownership of the exectuables return value into the Frame struct we achieve this.

A zero-copy implementation of those APIs would be technically possible without that internal change by making the callsite in the runtime responsible for moving the returned data into the frame after any call. However, resetting the stored output needs to be handled in ext, since plain transfers will not affect the stored return data (and we don't want to handle this special call case inside the runtime API). This has drawbacks:

It can not be tested easily in the mock.
It introduces an inconsistency where resetting the stored output is handled in ext, but the runtime API is responsible to store it back correctly after any calls made. Instead, with ownership of the data in Frame, both can be handled in a single place. Handling both in fn run() is more natural and leaves less room for runtime API bugs.

The returned output is reset each time before running any executable in a nested stack. This change should not incur any overhead to the overall memory usage as only the returned data from the last executed frame will be kept around at any time.

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Signed-off-by: xermicus <cyrill@parity.io>

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Signed-off-by: xermicus <cyrill@parity.io>

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Signed-off-by: xermicus <cyrill@parity.io>

xermicus · 2024-09-20T16:52:16Z

bot fmt

command-bot · 2024-09-20T16:52:22Z

@xermicus https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7398794 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 5-936aad13-b847-43cd-8d31-72960ecb926d to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-09-20T16:54:18Z

@xermicus Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7398794 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7398794/artifacts/download.

Signed-off-by: xermicus <cyrill@parity.io>

command-bot · 2024-09-24T12:51:01Z

@xermicus Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7419963 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7419963/artifacts/download.

substrate/frame/revive/src/wasm/runtime.rs

substrate/frame/revive/src/exec.rs

athei · 2024-09-24T14:37:59Z

substrate/frame/revive/src/exec.rs

 		} else {
-			Self::transfer_no_contract(&origin, &dest, value)
+			Self::transfer_no_contract(&origin, &dest, value).map(|_| Default::default())


Doesn't this already return ExecResult? No map necessary.

Yep good catch, fixed.

substrate/frame/revive/src/exec.rs

athei · 2024-09-24T14:44:28Z

substrate/frame/revive/src/exec.rs

-	) -> ExecResult {
+	) -> Result<(), ExecError> {


Should be the same?

The returned output is now attached to the frame, so we can't return it anymore, or am I missing something?

substrate/frame/revive/src/wasm/runtime.rs

… drop early Signed-off-by: xermicus <cyrill@parity.io>

substrate/frame/revive/src/exec.rs

Signed-off-by: xermicus <cyrill@parity.io>

This reverts commit b6c3705.

Signed-off-by: xermicus <cyrill@parity.io>

xermicus · 2024-09-25T08:26:29Z

bot fmt

command-bot · 2024-09-25T08:26:35Z

@xermicus https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7432068 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 5-a508ee1a-95b8-45e7-8af5-ad912219182d to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-09-25T08:28:23Z

@xermicus Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7432068 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7432068/artifacts/download.

substrate/frame/revive/src/wasm/runtime.rs

Co-authored-by: PG Herveou <pgherveou@gmail.com>

Signed-off-by: xermicus <cyrill@parity.io>

xermicus and others added 7 commits September 18, 2024 21:11

store return data in frame

6488586

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

impl runtime apis

1a7569c

Signed-off-by: xermicus <cyrill@parity.io>

Merge branch 'master' into cl/return-data-api

68fb924

better name for output field

d53dcae

Signed-off-by: xermicus <cyrill@parity.io>

uapi

5073e45

Signed-off-by: xermicus <cyrill@parity.io>

wip tests

81f77ef

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

test fixture

c981a75

Signed-off-by: xermicus <cyrill@parity.io>

xermicus added R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts. labels Sep 19, 2024

xermicus and others added 9 commits September 20, 2024 10:04

store the output directly

6f43354

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

executable output is owned by the frame

0eb213c

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

some bikeshedding

68e989b

Signed-off-by: xermicus <cyrill@parity.io>

fx

039c6b3

Signed-off-by: xermicus <cyrill@parity.io>

fix

f198cf5

Signed-off-by: xermicus <cyrill@parity.io>

test return data api on instantiate in the mock

8e8b2a2

Signed-off-by: xermicus <cyrill@parity.io>

test return data api on nested calls in the mock

4caa6a0

Signed-off-by: xermicus <cyrill@parity.io>

prdoc

a04cfb7

Signed-off-by: xermicus <cyrill@parity.io>

Merge branch 'master' into cl/return-data-api

d366111

xermicus marked this pull request as ready for review September 20, 2024 16:17

xermicus requested review from pgherveou, athei and smiasojed September 20, 2024 16:17

".git/.scripts/commands/fmt/fmt.sh"

fcacb89

xermicus added 4 commits September 23, 2024 13:39

Merge branch 'master' into cl/return-data-api

5878734

Merge branch 'master' into cl/return-data-api

f3ca36b

work around borrow checker limitations

b6c3705

Signed-off-by: xermicus <cyrill@parity.io>

Merge branch 'master' into cl/return-data-api

b212893

".git/.scripts/commands/fmt/fmt.sh"

7b69ee7

athei reviewed Sep 24, 2024

View reviewed changes

pgherveou reviewed Sep 24, 2024

View reviewed changes

substrate/frame/revive/src/wasm/runtime.rs Outdated Show resolved Hide resolved

substrate/frame/revive/src/wasm/runtime.rs Outdated Show resolved Hide resolved

remove take_caller_output helper and provide a comment why we want to…

1b83cf5

… drop early Signed-off-by: xermicus <cyrill@parity.io>

pgherveou reviewed Sep 24, 2024

View reviewed changes

substrate/frame/revive/src/exec.rs Show resolved Hide resolved

xermicus added 2 commits September 24, 2024 17:57

move sandbox write helpers into Memory trait

2e1c5fc

Signed-off-by: xermicus <cyrill@parity.io>

write actual buf len

8ea20a1

Signed-off-by: xermicus <cyrill@parity.io>

athei approved these changes Sep 24, 2024

View reviewed changes

xermicus added 3 commits September 25, 2024 10:01

Revert "work around borrow checker limitations"

a7a9c63

This reverts commit b6c3705.

rm stale methods

1e3cddf

Signed-off-by: xermicus <cyrill@parity.io>

Merge branch 'master' into cl/return-data-api

eeffa2d

".git/.scripts/commands/fmt/fmt.sh"

06305fe

xermicus requested a review from pgherveou September 25, 2024 08:33

pgherveou reviewed Sep 25, 2024

View reviewed changes

substrate/frame/revive/src/wasm/runtime.rs Outdated Show resolved Hide resolved

pgherveou approved these changes Sep 25, 2024

View reviewed changes

Update substrate/frame/revive/src/wasm/runtime.rs

456697c

Co-authored-by: PG Herveou <pgherveou@gmail.com>

xermicus enabled auto-merge September 25, 2024 11:21

xermicus disabled auto-merge September 25, 2024 11:22

xermicus enabled auto-merge September 25, 2024 11:24

xermicus disabled auto-merge September 25, 2024 11:27

fix return types

61f1f1e

Signed-off-by: xermicus <cyrill@parity.io>

xermicus added this pull request to the merge queue Sep 25, 2024

Merged via the queue into master with commit c77095f Sep 25, 2024
221 checks passed

xermicus deleted the cl/return-data-api branch September 25, 2024 17:05

xermicus mentioned this pull request Sep 27, 2024

pallet_revive launch tracking issue #5308

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pallet-revive] last call return data API #5779

[pallet-revive] last call return data API #5779

xermicus commented Sep 19, 2024 •

edited

Loading

xermicus commented Sep 20, 2024

command-bot bot commented Sep 20, 2024 •

edited

Loading

command-bot bot commented Sep 20, 2024

command-bot bot commented Sep 24, 2024

athei Sep 24, 2024

xermicus Sep 25, 2024

athei Sep 24, 2024

xermicus Sep 25, 2024

xermicus commented Sep 25, 2024

command-bot bot commented Sep 25, 2024 •

edited

Loading

command-bot bot commented Sep 25, 2024

[pallet-revive] last call return data API #5779

[pallet-revive] last call return data API #5779

Conversation

xermicus commented Sep 19, 2024 • edited Loading

xermicus commented Sep 20, 2024

command-bot bot commented Sep 20, 2024 • edited Loading

command-bot bot commented Sep 20, 2024

command-bot bot commented Sep 24, 2024

athei Sep 24, 2024

Choose a reason for hiding this comment

xermicus Sep 25, 2024

Choose a reason for hiding this comment

athei Sep 24, 2024

Choose a reason for hiding this comment

xermicus Sep 25, 2024

Choose a reason for hiding this comment

xermicus commented Sep 25, 2024

command-bot bot commented Sep 25, 2024 • edited Loading

command-bot bot commented Sep 25, 2024

xermicus commented Sep 19, 2024 •

edited

Loading

command-bot bot commented Sep 20, 2024 •

edited

Loading

command-bot bot commented Sep 25, 2024 •

edited

Loading